fix: fix optimize-locales-plugin not "tree shaking" react-aria package#10023
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Expands the locales optimization path filter to include the react-aria package, fixing cases where the optimize-locales tooling wasn’t applied (and thus preventing expected tree-shaking / bundle reduction).
Changes:
- Extend the resolver’s
dependency.sourcePathregex to matchreact-aria. - Extend the Vite/Rollup unplugin
resolveIdregex to matchreact-aria.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/dev/parcel-resolver-optimize-locales/LocalesResolver.js | Updates source path matching so Parcel resolver also applies to react-aria. |
| packages/dev/optimize-locales-plugin/LocalesPlugin.js | Updates plugin resolution filtering so the unplugin applies to react-aria. |
| }, | ||
| resolve({specifier, dependency, config}) { | ||
| if (!Array.isArray(config) || !/[/\\](@react-stately|@react-aria|@react-spectrum|react-aria-components)[/\\]/.test(dependency.sourcePath)) { | ||
| if (!Array.isArray(config) || !/[/\\](@react-stately|@react-aria|@react-spectrum|react-aria|react-aria-components)[/\\]/.test(dependency.sourcePath)) { |
| name: 'locales-plugin', | ||
| resolveId(specifier, sourcePath, options) { | ||
| if (!/[/\\](@react-stately|@react-aria|@react-spectrum|react-aria-components)[/\\]/.test(sourcePath) || options?.ssr) { | ||
| if (!/[/\\](@react-stately|@react-aria|@react-spectrum|react-aria|react-aria-components)[/\\]/.test(sourcePath) || options?.ssr) { |
snowystinger
left a comment
snowystinger
left a comment
There was a problem hiding this comment.
I'm having trouble reproducing, as mentioned here #10022 (comment)
but this does look correct
snowystinger
left a comment
snowystinger
left a comment
There was a problem hiding this comment.
Tested by going to our example app and running build before and after this change.
|
Ah, looks like there was a merge conflict, you'll want to resolve it by choosing your changes and then running |
|
@snowystinger Here ya go! Congrats on oxfmt migration. |
|
That got me thinking, should the package version be bumped manually? |
yihuiliao
added
the
no testing
Closes #10022
✅ Pull Request Checklist:
📝 Test Instructions:
Monkey patch any project using optimize-locales-plugin with the same patch. Observe:
Before:
After:
4.93 KB savings just like that!
🧢 Your Project: